-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
replace duplicated Bitmaps with an powerful already importd one #211
base: main
Are you sure you want to change the base?
Conversation
@yuhuajing we are discussing the merits of your PR. One decision we have made is that we won't include IDE specific files in the repo. As such, could you update your PR to gitignore the whole .idea directory, and remove the associated files from your PR |
A result of the change beyond flatten files possibly being easier, is that the resulting contract should be smaller. ERC721Psi calls set and get on solidity-bits version of BitMaps, where as ImmutableERC721Base uses the same functions in Open Zeppelin's code. This means that two copies of these functions will be included in the final contract bytecode. Making the change described in this PR will mean there is only one copy of the set and get functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please .gitignore the whole .idea directory, and remove .idea files. We have decided not to include IDE specific files in the repo.
replace duplicated Bitmaps with an powerful already importd one
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
5ed7878
to
3b75c53
Compare
Already done |
The flattern files will faile because the duplicated BitMaps import, thus we strongly suggest that you could replace the imported BitMaps library in the /contracts/token/erc721/abstract/ERC721Hybrid.sol with
_import {BitMaps} from "solidity-bits/contracts/BitMaps.sol";
in the \contracts\token\erc721\erc721psi\ERC721PsiBurnable.sol.